-
Notifications
You must be signed in to change notification settings - Fork 118
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add checksum-md5
command
#536
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding this feature! We should also add the subcommand for cargo-espflash
The initial idea of the Rust stub, was to put them behind a feature, but we decided not to do that because nobody will use the feature. That being said, we now have an argument to do so or to go back to the C stubs or just force to use |
BTW I also currently cannot flash the H2 with the current stub - we really need to be very confident that the stub works for us (including MD5) before we force every user to use it (while I agree that no average user will use such a feature we could agree to use it in our team - it's kind of a chicken and egg problem) Update: same for C6 |
Current |
In any case, that behind the scope of this PR, I'll do some testing tomorrow morning and update with the results! |
I locally updated the flasher stubs ( commit 2b0b0f0de5020e6d5e2bf39e8950d49737fd5edb ) and it works for all chips - also flashing H2 works now for me I'm still not sure if we should add a feature to choose the old vs the new stub. e.g. the new stub doesn't support 26 MHz xtals - maybe having at least a feature to force using the old stubs might be helpful for those users (but certainly not scope of this PR) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Tested it on all the targets but S2 and ESP32 and worked fine with the new stubs, will create a PR updating them!
Edit: PR updating the stubs: #537
Is the 26 MHz a stub issue? I don't have any 26 MHz board so I cant test it. We have #526, but that's a bootloader issue. This definitely needs some investigation, so I'll try to get some 26 MHz devkit |
Yes it works with |
This adds a
checksum-md5
command to espflash.e.g.
While this is not too exciting on its own, it's the ground work for #259 and #479
It's very unfortunate that the Rust flasher-stub currently doesn't implement the required command while the ROM code and the C based flasher stub does. (Verified by temporarily reverting the stubs locally). Maybe we should revert the change to use the Rust flasher-stub (but use it based on a feature at build time) until it's supported there
I checked the result with reading a flash region with
esptool
and runningmd5sum
on it.